[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 #29

HuangZhenQiu · 2024-08-11T02:37:27Z

Bump Flink version to 1.20.0 to prepare support lineage in Cassandra connector

boring-cyborg · 2024-08-11T02:37:31Z

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

echauchot

Sorry for the delay. I finally looked at this PR

echauchot · 2024-10-01T12:49:21Z

.github/workflows/push_pr.yml

+        flink: [ 1.20.0 ]
        include:
+          - flink: 1.19.0


The rule is the test the connector with the last 2 major Flink versions so 1.20.0 and 1.19.1. And for readability I think this it is better to use the matrix
flink: [1.20.0, 1.19.1]

Also, you just updated the github action job that reacts to PR pushes. But you need to update the weekly.yml file which is the main job running every sunday

In this file I'd test released (v3.2) branch against last 2 snapshots of flink (to check that ongoing iterations of flink to not break released version of the connector) and the main branch against last 2 relesed versions of flink (to check that the current iteration of the connector still works on released flink versions)

echauchot · 2024-10-01T13:19:35Z

flink-connector-cassandra/archunit-violations/dcfaa83d-a12c-48e1-9e51-b8d3808cd287

@@ -1,10 +1,4 @@
 Constructor <org.apache.flink.connector.cassandra.source.CassandraSource.<init>(org.apache.flink.streaming.connectors.cassandra.ClusterBuilder, long, java.lang.Class, java.lang.String, org.apache.flink.streaming.connectors.cassandra.MapperOptions)> calls method <org.apache.flink.api.java.ClosureCleaner.clean(java.lang.Object, org.apache.flink.api.common.ExecutionConfig$ClosureCleanerLevel, boolean)> in (CassandraSource.java:138)
-Constructor <org.apache.flink.connector.cassandra.source.CassandraSource.<init>(org.apache.flink.streaming.connectors.cassandra.ClusterBuilder, long, java.lang.Class, java.lang.String, org.apache.flink.streaming.connectors.cassandra.MapperOptions)> calls method <org.apache.flink.util.Preconditions.checkNotNull(java.lang.Object, java.lang.String)> in (CassandraSource.java:124)


These archunit violations were legitimate to store in the violation store as they are accepted. Now that you removed them you have build issues.
Take a look at the comments in file archunit.properties to see how to use it.

It is the same reason as I removed WriteAheadSinkTestBase as the base class of CassandraConnectorITCase.

echauchot · 2024-10-01T13:31:14Z

.../src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java

-        extends WriteAheadSinkTestBase<
-                Tuple3<String, Integer, Integer>,
-                CassandraTupleWriteAheadSink<Tuple3<String, Integer, Integer>>> {
+class CassandraConnectorITCase {



Don't remove this inheritance otherwise you no more test what's in WriteAheadSinkTestBase

echauchot · 2024-10-01T13:42:44Z

...ssandra/src/test/java/org/apache/flink/connector/cassandra/source/CassandraSourceITCase.java

@@ -197,36 +196,31 @@ protected void checkResultWithSemantic(
    }

    @Disabled("Not a unbounded source")
-    @Override


I prefer keeping the overrides as these methods are indeed defined in the parent class even though they are disabled because related to streaming source

Agree. I guess it is auto removed after removing WriteAheadSinkTestBase as base class.

No, these methods come from the source base test suite.

echauchot · 2024-10-01T13:43:09Z

.../src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java

@@ -284,7 +280,6 @@ void testAnnotatePojoWithTable() {
    //  Exactly-once Tests
    // ------------------------------------------------------------------------

-    @Override
    protected CassandraTupleWriteAheadSink<Tuple3<String, Integer, Integer>> createSink()


and keep the overrides.

echauchot · 2024-10-01T13:44:15Z

pom.xml

@@ -42,7 +42,7 @@ under the License.
 	</scm>

 	<properties>
-		<flink.version>1.18.0</flink.version>
+		<flink.version>1.20.0</flink.version>


you also need to fix the depencies convergence issues with 1.20.0

Dependency convergence error for org.apache.commons:commons-lang3:3.12.0 paths to dependency are: +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-streaming-java:1.20.0 +-org.apache.flink:flink-core:1.20.0 +-org.apache.commons:commons-lang3:3.12.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-streaming-java:1.20.0 +-org.apache.flink:flink-core:1.20.0 +-org.apache.commons:commons-text:1.10.0 +-org.apache.commons:commons-lang3:3.12.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-streaming-java:1.20.0 +-org.apache.flink:flink-core:1.20.0 +-org.apache.commons:commons-compress:1.26.0 +-org.apache.commons:commons-lang3:3.14.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-streaming-java:1.20.0 +-org.apache.flink:flink-runtime:1.20.0 +-org.apache.commons:commons-lang3:3.12.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-streaming-java:1.20.0 +-org.apache.flink:flink-java:1.20.0 +-org.apache.commons:commons-lang3:3.12.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-test-utils:1.20.0 +-org.apache.flink:flink-runtime:1.20.0 +-org.apache.commons:commons-lang3:3.12.0 and +-org.apache.flink:flink-connector-cassandra_2.12:4.0-SNAPSHOT +-org.apache.flink:flink-test-utils:1.20.0 +-org.apache.flink:flink-core:1.20.0 +-org.apache.commons:commons-lang3:3.12.0

HuangZhenQiu · 2024-10-01T17:46:13Z

@echauchot
Thanks for the review. The major reason of removing WriteAheadSinkTestBase as the base class of CassandraConnectorITCase is WriteAheadSinkTestBase is no more accessible out side of the package "org.apache.flink.streaming.runtime.operators".

Shall I remove the CassandraConnectorITCase to package namespace org.apache.flink.streaming.runtime.operators?

echauchot · 2024-10-10T09:40:55Z

@echauchot Thanks for the review. The major reason of removing WriteAheadSinkTestBase as the base class of CassandraConnectorITCase is WriteAheadSinkTestBase is no more accessible out side of the package "org.apache.flink.streaming.runtime.operators".

Shall I remove the CassandraConnectorITCase to package namespace org.apache.flink.streaming.runtime.operators?

no I don't think the cassandra test should be moved to a flink core namespace. It looks strange to me that the visibility of WriteAheadSinkTestBase was reduced as part of the migration to Junit 5, so I commented in the ticket were the change was made.

boring-cyborg bot added the component=Connectors/Cassandra label Aug 11, 2024

HuangZhenQiu force-pushed the FLINK-34468-bump-flink-version branch from a66b8d4 to a4af9a1 Compare August 21, 2024 22:09

boring-cyborg bot added the component=BuildSystem label Aug 21, 2024

HuangZhenQiu changed the title ~~[FLINK-34468] bump flink version to 1.20.0~~ [FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 Aug 21, 2024

[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20

908240d

HuangZhenQiu force-pushed the FLINK-34468-bump-flink-version branch from a4af9a1 to 908240d Compare August 21, 2024 22:10

echauchot requested changes Oct 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 #29

[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 #29

HuangZhenQiu commented Aug 11, 2024

boring-cyborg bot commented Aug 11, 2024

echauchot left a comment

echauchot Oct 1, 2024

echauchot Oct 1, 2024

echauchot Oct 1, 2024

HuangZhenQiu Oct 1, 2024

echauchot Oct 1, 2024

echauchot Oct 1, 2024

HuangZhenQiu Oct 1, 2024

echauchot Oct 10, 2024

echauchot Oct 1, 2024

echauchot Oct 1, 2024

HuangZhenQiu commented Oct 1, 2024 •

edited

Loading

echauchot commented Oct 10, 2024

		@@ -1,10 +1,4 @@
		Constructor <org.apache.flink.connector.cassandra.source.CassandraSource.<init>(org.apache.flink.streaming.connectors.cassandra.ClusterBuilder, long, java.lang.Class, java.lang.String, org.apache.flink.streaming.connectors.cassandra.MapperOptions)> calls method <org.apache.flink.api.java.ClosureCleaner.clean(java.lang.Object, org.apache.flink.api.common.ExecutionConfig$ClosureCleanerLevel, boolean)> in (CassandraSource.java:138)
		Constructor <org.apache.flink.connector.cassandra.source.CassandraSource.<init>(org.apache.flink.streaming.connectors.cassandra.ClusterBuilder, long, java.lang.Class, java.lang.String, org.apache.flink.streaming.connectors.cassandra.MapperOptions)> calls method <org.apache.flink.util.Preconditions.checkNotNull(java.lang.Object, java.lang.String)> in (CassandraSource.java:124)

[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 #29

Are you sure you want to change the base?

[FLINK-34468][Connector/Cassandra] Adding support for Flink 1.20 #29

Conversation

HuangZhenQiu commented Aug 11, 2024

boring-cyborg bot commented Aug 11, 2024

echauchot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuangZhenQiu commented Oct 1, 2024 • edited Loading

echauchot commented Oct 10, 2024

HuangZhenQiu commented Oct 1, 2024 •

edited

Loading